Skip to content

[SREP-1045] Develop Clusterrole webhook #379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 11, 2025

Conversation

diakovnec
Copy link
Contributor

@diakovnec diakovnec commented Aug 8, 2025

This PR is for implementing a new clusterrole webhook. We have a way to test it for Rosa Classic, but it is complicated for HCP because of image embedding and ACM policies that deploy the package to the HCP namespace.

One of the possible ways is to merge PR into the stage and verify if it provisions all resources as expected.

The functionality of the webhook itself has been verified for Rosa Classic and theoretically should work for HCP as long as it's enabled with func (s *ClusterRoleWebHook) HypershiftEnabled() bool { return true }

Also, the purpose of this PR is to check the e-2-e pipeline process.

Note: more context in the Slack Discussion
Test for Classic Rosa

 oc delete clusterrole system:node
Error from server (Forbidden): admission webhook "clusterroles-validation.managed.openshift.io" denied the request: Deleting ClusterRole system:node is not allowed
I0808 01:04:11.343741       1 clusterrole.go:122] "msg"="Found clusterrole: system:node" "logger"="clusterroles-validation"
I0808 01:04:11.343760       1 clusterrole.go:127] "msg"="Deleting operation detected on ClusterRole: system:node" "logger"="clusterroles-validation"

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 8, 2025
@openshift-ci openshift-ci bot requested review from anispate and Dee-6777 August 8, 2025 01:25
@diakovnec diakovnec changed the title [WIP] [SREP-1045] Clusterrole webhook testing in stage [SREP-1045] Clusterrole webhook testing in stage Aug 8, 2025
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 8, 2025
@diakovnec diakovnec changed the title [SREP-1045] Clusterrole webhook testing in stage [SREP-1045] Develop Clusterrole webhook Aug 8, 2025
}

// Protected ClusterRoles that should not be deleted
protectedClusterRoles = []string{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a full list? it mentioned backplane-* in the description.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The full list will contain any backplane related clusterroles + most openshift critical. Its not fiesible to include all system:* roles as they can change over time, and we still want customers being able to create and remove their own clusterroles. The idea is being able to get access to the cluster if some clusterroles have been deleted.

Copy link
Contributor

openshift-ci bot commented Aug 8, 2025

@diakovnec: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@bmeng
Copy link
Contributor

bmeng commented Aug 11, 2025

/label tide/merge-method-squash

@openshift-ci openshift-ci bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Aug 11, 2025
@bmeng
Copy link
Contributor

bmeng commented Aug 11, 2025

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2025
Copy link
Contributor

openshift-ci bot commented Aug 11, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bmeng, diakovnec

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 11, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit 21222a6 into openshift:master Aug 11, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants